Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate core functions and remove sphinx style argument types #205

Merged
merged 33 commits into from
Aug 4, 2023

Conversation

tonybaloney
Copy link
Owner

@tonybaloney tonybaloney commented Aug 1, 2023

Changes:

  • Operator Metric and Archiver are now generics
  • Uses pyright to check
  • Pins tabulate and gitpython versions that require specific flags we're using
  • Removes docstring types

there are possible more places to add types and some violations reported by MyPy, but this is a good start

@codecov-commenter
Copy link

codecov-commenter commented Aug 1, 2023

Codecov Report

Merging #205 (477dd77) into master (d501d01) will increase coverage by 0.03%.
Report is 2 commits behind head on master.
The diff coverage is 96.38%.

@@            Coverage Diff             @@
##           master     #205      +/-   ##
==========================================
+ Coverage   95.42%   95.46%   +0.03%     
==========================================
  Files          24       26       +2     
  Lines        1269     1346      +77     
  Branches      287      287              
==========================================
+ Hits         1211     1285      +74     
- Misses         33       37       +4     
+ Partials       25       24       -1     
Files Changed Coverage Δ
src/wily/decorators.py 0.00% <0.00%> (ø)
src/wily/commands/list_metrics.py 90.90% <50.00%> (ø)
src/wily/archivers/__init__.py 94.11% <88.46%> (-5.89%) ⬇️
src/wily/operators/__init__.py 97.80% <95.00%> (-2.20%) ⬇️
src/wily/cache.py 98.21% <95.45%> (+0.03%) ⬆️
src/wily/config/types.py 97.50% <97.50%> (ø)
src/wily/__init__.py 100.00% <100.00%> (ø)
src/wily/__main__.py 95.69% <100.00%> (ø)
src/wily/archivers/filesystem.py 100.00% <100.00%> (ø)
src/wily/archivers/git.py 87.65% <100.00%> (+0.15%) ⬆️
... and 12 more

Copy link
Collaborator

@devdanzin devdanzin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I've only found minor nits, some even in code that didn't change in this PR. Among them, some missed docstring types, a couple of places where typing could be improved and some places where we could optionally add return types. Nothing that should get in the way of merging.

This is a great start and I'd be glad to tackle any improvements from here on if you'd like me to.

src/wily/archivers/__init__.py Outdated Show resolved Hide resolved
src/wily/archivers/__init__.py Outdated Show resolved Hide resolved
src/wily/archivers/__init__.py Outdated Show resolved Hide resolved
@@ -107,7 +113,7 @@ def revisions(self, path: str, max_revisions: int) -> List[Revision]:
if self.repo.is_dirty():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some typing information remains in the above docstring and that on find().

src/wily/cache.py Show resolved Hide resolved
src/wily/helper/__init__.py Outdated Show resolved Hide resolved
src/wily/helper/__init__.py Outdated Show resolved Hide resolved
src/wily/helper/__init__.py Outdated Show resolved Hide resolved
src/wily/operators/__init__.py Show resolved Hide resolved
src/wily/state.py Outdated Show resolved Hide resolved
@tonybaloney tonybaloney merged commit 8eb4416 into master Aug 4, 2023
@tonybaloney tonybaloney deleted the typing_updates branch August 4, 2023 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants